-
Notifications
You must be signed in to change notification settings - Fork 45
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fixes for #165, #238, #210, #205 #241
base: master
Are you sure you want to change the base?
Conversation
@larrybradley will you look at the changes I made for the modeling and make sure I'm not doing something stupid? |
@sosey The Gaussian model (red line) in your figure above looks (by eye) to have a mean/center value of r > 0 (it appears to be sloping downward to the left at r=0). It should be fixed to have a mean/center at exactly r=0. |
Is this where I admit my ignorance, that I don't know how to check out a branch that addresses an issue? |
Hi - I wanted to check on updates to imexam to get correct radial profile FWHM's from 'r' command. How does one update from 0.9.1 to test? |
fixed = {'mean': True, 'amplitude': True} | ||
bounds = {'amplitude': (flux.max() / 2., flux.max())} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I copied this setting to spacetelescope/jdaviz#1409
This PR attempts to fix issues with the 1D gaussian fits used in radial plots and line plots. As such it addresses some common issues in the following:
fixes #165
closes #238
closes #210
closes #205
I still need to clean up and there's likely some more minor changes to make before this PR is merged, but FWHM looks more appropriate.
@janerigby checkout this branch and let me know if you see improvement
![image](https://user-images.githubusercontent.com/503615/137814328-fa715789-3bf5-44cf-8262-14ad7ce8bc0d.png)